Clean Streaming Code

Streaming is an awesome tool. It helps to separate concerns (e.g., iteration, filtering, execution), to avoid state and thus improves thread safety. Additionally, we can express the same functionality with less code. But it’s hard to read and to test.

Really?

In a recent project, we introduced Java 8 streaming to legacy code and converted a lot of loops. Although I’m quite used to writing code that makes use of Java 8 streaming API, I realized how actually hard it was to read such a code written by others (and thus how hard my code might be to read by others). Additionally, there were bugs in the code even though the tests were all green and were covering 100% of the lines. These tests were quite complicated as they had to test the whole method including iteration, filtering and execution, so a lot of different concerns.

The following shows the legacy code.

Legacy Code

private void addLinksToTarget(List<Combination> links, Combination combination, String caseDesc) {

    for (Iterator<Combination> linksIterator = links.iterator(); links.hasNext(); combination.getTargets().size() < PLACES_PER_KEYWORD) {
        Combination link = linkIterator.next();
        if (!link.getSegment().equals(combination.getSegment())) {
            continue;
        }

        Rating linkLimit = link.getRatingByType(RatingType.AVAILABLE_PLACES);

        if (linkLimit.getValue() <= 0) {
            linkIterator.remove();
            continue;
        }
        // avoid duplicate urls in the same footer
        String sourceUrl = urlFormatter.format(combination);
        String targetUrl = urlFormatter.format(link);
        if (targetUrl.equals(sourceUrl)) {
            continue;
        }
        boolean alreadyContained = false;
        for (String footerPart : combination.getTargets()) {
            if (StringUtils.containsIgnoreCase(footerPart, targetUrl)) {
                alreadyContained = true;
                break;
            }
        }
        if (alreadyContained) {
            continue;
        }
        String target = link.getLabel()
                        + ">" + targetUrl
                        + ">" + link.getPriority()
                        + ">" + caseDesc;
        combination.getTargets().add(target);
        linkLimit.setValue(linkLimit.getValue() - 1);
    }
}

A fellow dev of mine took the challenge and transformed the original version to the following using Java 8 streaming.

Streaming First Draft

private void addLinksToTarget(List<Combination> links, Combination combination, String caseDesc) {
    links
        .stream()
        .filter(link -> link.getSegment().equals(combination.getSegment()))
        .filter(link -> link.getRatingByType(AVAILABLE_PLACES)
        .orElse(new Rating(AVAILABLE_PLACES, 0)).getValue() > 0)
        .filter(link -> !isSelfReference(combination, link))
        .filter(link -> !isAlreadyContainedInTargets(link, combination))
        .limit(linksPerCombination - combination.getTargets().size())
        .forEach(link -> {
            String targetUrl = urlFormatter.format(placeable);
            String target = link.getLabel()
            + ">" + targetUrl
            + ">" + link.getPriority()
            + ">" + caseDesc;
            combination.getTargets().add(target);
            Rating linkLimit = link.getRatingByType(AVAILABLE_PLACES).get();
            linkLimit.setValue(linkLimit.getValue() - 1);
            });
        links.removeIf(link -> {
            Optional<Rating> rating = link.getRatingByType(AVAILABLE_PLACES);
            return !rating.isPresent() || rating.get().getValue() <= 0;
        });
}

This code now looks much better, but it’s still hard to read – at least was for me as an outsider. So I started refactoring the code to a more readable form. I created predicates containing the filter logic and gave them meaningful names and extracted the business logic executed in forEach to a consumer.

Streaming cleaned up

private void addLinksToTarget(List<Combination> links, Combination combination, String caseDesc) {
    Helper helper = Helper.of(combination, urlFormatter, caseDesc);
    links
        .stream()
        .filter(helper.sameSegments)
        .filter(helper.hasLinkPlacesLeft)
        .filter(helper.notSelfReference)
        .filter(helper.linkNotContained)
        .limit(helper.maxAvailableLinks)
        .forEach(helper.addLinkToTargetWithCaseDescription);
    links.removeIf(helper.noLinkPlacesLeft);
}

Every condition and the consumer are now easily (and isolated) testable and possible bugs could be found faster. But most important: I easily can understand what this code block is intended to do, even if I wasn’t the author.

Conclusion

We learned great things from Uncle Bob like moving blocks of logic into self-describing methods or not being done before the code is refactored to be well readable by others (including my future self in half a year). Apparently, we sometimes forget these learnings when it comes to new features like the streaming API of Java 8. Maybe it’s because we’re so excited to have them and think it’s all much clearer now – or simply because we need to gather some more experience.

This blog post has also been published on the eBay technology blog (DE)